-
Notifications
You must be signed in to change notification settings - Fork 36
Add automated security scanning workflows #124
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
This pull request sets up GitHub code scanning for this repository. Once the scans have completed and the checks have passed, the analysis results for this pull request branch will appear on this overview. Once you merge this pull request, the 'Security' tab will show more code scanning analysis results (for example, for the default branch). Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results. For more information about GitHub code scanning, check out the documentation. |
- Scan LOW,MEDIUM,HIGH,CRITICAL instead of only HIGH,CRITICAL - Remove Docker image scan (no :latest tag exists)
Enable vuln, secret, and misconfig scanners explicitly
- Build Go binary for linux/amd64 - Build Docker image with buildx - Scan the built image (not filesystem) - Matches coder/coder scanning approach
- Add table format scan to show results in workflow logs - Upload SARIF as artifact for manual inspection - Matches coder/coder artifact upload pattern
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds comprehensive automated security scanning workflows to improve supply chain security for air-gapped deployments. The changes include CodeQL vulnerability scanning, Trivy image scanning, OpenSSF Scorecard security assessment, and enhanced Dependabot configuration.
- Automated daily security scanning with CodeQL and Trivy
- Weekly OpenSSF Scorecard security best practices assessment
- Enhanced Dependabot with commit prefixes and patch update filtering
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
File | Description |
---|---|
CHANGELOG.md | Documents the addition of security scanning workflows and Dependabot enhancements |
.github/workflows/security.yaml | Main security workflow with CodeQL and Trivy scanning jobs |
.github/workflows/scorecard.yml | OpenSSF Scorecard workflow for security best practices assessment |
.github/dependabot.yaml | Enhanced configuration with commit prefixes and patch update filtering |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Makefile
Outdated
|
||
TAG=$(shell git describe --always) | ||
|
||
build/linux/amd64: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
build/linux/amd64: | |
bin/coder-marketplace-linux-amd64: |
You can keep the PHONY (after editing the target name) for simplicity's sake though, otherwise you'll need to specify every Go-related file as a dependency
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
would it be better to leave PHONY or to use a pattern like one of the following to ensure Make can still optimize by not building if no Go files change?
bin/code-marketplace-linux-amd64: $(wildcard **/*.go) go.mod go.sum
If you think leaving PHONY is simpler and cleaner then I am open just like using Make to optimize builds even in small repos like this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you want to do it you'll probably have to use shell find
cuz I don't think make's wildcard
is very good. But it's probably fine to just leave it as PHONY for this PR
@code-asher could you give this a review and flag anything that's weird for you? I haven't contributed to this repo before so maybe I'm missing something in my reviews |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome thank you!
.github/dependabot.yaml
Outdated
# Ignore patch updates for all dependencies to reduce PR noise | ||
- dependency-name: "*" | ||
update-types: | ||
- version-update:semver-patch |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need this? So far the number of updates seem to have been pretty mild. Or we could group updates weekly or something.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
grouped weekly. removed ignore
.github/dependabot.yaml
Outdated
commit-message: | ||
prefix: "ci" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not a huge deal, but I think the prefixes may have no use, the changelog is manually curated.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
agreed and removed
.github/dependabot.yaml
Outdated
commit-message: | ||
prefix: "chore" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ditto
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
agreed and removed
scorecard.yml:24: actions/checkout → v5.0.0 scorecard.yml:29: ossf/scorecard-action → v2.4.3 security.yaml:32: actions/checkout → v5.0.0 (CodeQL job) security.yaml:57: actions/checkout → v5.0.0 (Trivy job) security.yaml:81: aquasecurity/trivy-action → v0.33.1 security.yaml:88: aquasecurity/trivy-action → v0.33.1
removed PHONY alias added wildcard for .go files updated security workflow to use explicit build target vs old alias
removed patch ignore and instead we are grouping all-dependencies updates weekly
|
||
build/linux/amd64: | ||
# Individual build targets for each OS/arch combination | ||
bin/code-marketplace-mac-amd64: $(wildcard **/*.go) go.mod go.sum |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does wildcard **/*.go
work? I can't find any example of this online, seems like people do a lot of workarounds for this.
In coder/coder
we define a variable with the result of a $(shell find ...)
command
build/linux/amd64: | ||
# Individual build targets for each OS/arch combination | ||
bin/code-marketplace-mac-amd64: $(wildcard **/*.go) go.mod go.sum | ||
mkdir -p bin |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You could maybe just put a $(shell mkdir -p bin)
at the top of the file (not in a target) to avoid having to duplicate it in every target
CGO_ENABLED=0 GOOS=linux GOARCH=arm64 go build -ldflags "-X github.com/coder/code-marketplace/buildinfo.tag=$(TAG)" -o bin/code-marketplace-linux-arm64 ./cmd/marketplace/main.go | ||
CGO_ENABLED=0 GOOS=windows GOARCH=amd64 go build -ldflags "-X github.com/coder/code-marketplace/buildinfo.tag=$(TAG)" -o bin/code-marketplace-windows-amd64 ./cmd/marketplace/main.go | ||
CGO_ENABLED=0 GOOS=windows GOARCH=arm64 go build -ldflags "-X github.com/coder/code-marketplace/buildinfo.tag=$(TAG)" -o bin/code-marketplace-windows-arm64 ./cmd/marketplace/main.go | ||
CGO_ENABLED=0 GOOS=darwin GOARCH=amd64 go build -ldflags "-X github.com/coder/code-marketplace/buildinfo.tag=$(TAG)" -o $@ ./cmd/marketplace/main.go |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If possible would be nice to make the recurring flags here a make variable
Closes #123
Adds automated security scanning to improve supply chain security for air-gapped deployments:
Changes
All scan results are uploaded to GitHub Security tab for centralized monitoring.
Testing